-
Notifications
You must be signed in to change notification settings - Fork 563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tests/main/sudo-env: check snap path under sudo #8875
Conversation
Some distributions set secure_path in /etc/sudoers which resets the PATH under sudo to some predefined set of locations. Make sure to account for all distros supported by snapd that have sudo set up this way. Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
c58d713
to
809bc87
Compare
tests/main/sudo-env/task.yaml
Outdated
. "$TESTSLIB/dirs.sh" | ||
|
||
# run a snap command via sudo | ||
su -l -c "sudo sh -c 'echo :\$PATH:'" test > sudo.path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use tests.session, su -l will give you a login shell and may trigger PAM.
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
tests.session -u test exec sudo sh -c 'echo :$PATH:' test > sudo.path | ||
# and again via sudo --login which should load /etc/profile | ||
# shellcheck disable=SC2016 | ||
tests.session -u test exec sudo --login sh -c 'echo :$PATH:' > sudo-login.path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed this part before. I wonder if this would cause root's session to start (and tear down in a moment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, if it causes problems we can revisit
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
tests/main/sudo-env/task.yaml
Outdated
|
||
# run a snap command via sudo | ||
# shellcheck disable=SC2016 | ||
tests.session -u test exec sudo sh -c 'echo :$PATH:' test > sudo.path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's with the errant test
towards the end here? I am having a difficult time telling which command that is an argument too, I think this would be more clear with some --
sprinkled in. ❇️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this
Some distributions set secure_path in /etc/sudoers which resets the PATH
under sudo to some predefined set of locations. Make sure to account for all
distros supported by snapd that have sudo set up this way.